Skip to content

Comments

ext/pcntl: Fix signal table updated before php_signal4 succeeds in pc…#21270

Closed
devnexen wants to merge 2 commits intophp:PHP-8.4from
devnexen:pcntl_signal_fix
Closed

ext/pcntl: Fix signal table updated before php_signal4 succeeds in pc…#21270
devnexen wants to merge 2 commits intophp:PHP-8.4from
devnexen:pcntl_signal_fix

Conversation

@devnexen
Copy link
Member

…ntl_signal

Move the signal table update after the php_signal4 call, mirroring what is already done in the SIG_DFL/SIG_IGN (integer) code path. This prevents a stale entry in the table if sigaction fails.

@devnexen devnexen marked this pull request as ready for review February 21, 2026 04:50
Copy link
Member

@arnaud-lb arnaud-lb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

I was wondering whether this introduced a race condition when a signal is delivered before the zend_hash_index_update(), but I think it doesn't since the handler doesn't touch &PCNTL_G(php_signal_table).

handle = zend_hash_index_update(&PCNTL_G(php_signal_table), signo, handle);
Z_TRY_ADDREF_P(handle);

/* we need to register in the OS side first before we update the internal list */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be worth it to specify why

…ntl_signal

Move the signal table update after the php_signal4 call, mirroring
what is already done in the SIG_DFL/SIG_IGN (integer) code path.
This prevents a stale entry in the table if sigaction fails.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants